fix: cachedFetcher.update() no longer blocks token reads during DB refresh by NETIZEN-11 · Pull Request #1535 · hyperledger-labs/fabric-token-sdk

NETIZEN-11 · 2026-04-12T19:22:32Z

What was fixed

The cachedFetcher.update() function in token/services/selector/sherdlock/fetcher.go was holding a write lock for the entire duration of the database query. This meant whenever the cache needed to refresh (e.g., on a slow DB), all token read operations would block waiting for the lock.

How it was fixed

Released the lock before executing the DB query
Added a staleness re-check after the DB call completes, to avoid overwriting a cache that another goroutine may have already refreshed
The cache update still happens atomically once the lock is re-acquired

Fix: #1547

Tests added

TestCachedFetcher_UpdateDoesNotBlockReaders: verifies that concurrent readers can still acquire the lock while update() is waiting on the database
TestCachedFetcher_UpdateReacquiresLockAfterDB: verifies the cache is correctly updated after the lock is re-acquired

go test -v -run "TestCachedFetcher_UpdateDoesNotBlockReaders|TestCachedFetcher_UpdateReacquiresLockAfterDB" ./token/services/selector/sherdlock/...

NETIZEN-11 · 2026-04-12T19:59:06Z

@adecaro Would love a review on this. Thanks!

adecaro · 2026-04-13T04:29:29Z

Hi @NETIZEN-11 , thanks for submitting this 🙏
I'll review ASAP but, please, open a Github Issue related to this.

Thanks much 🙏

adecaro · 2026-04-13T04:30:47Z

@NETIZEN-11 , please, run make lint-auto-fix. It should solve the current lint errors 🙏

NETIZEN-11 · 2026-04-13T16:59:00Z

@adecaro implemented the fix. Would love a review!

adecaro

LGTM. Thanks a lot @NETIZEN-11

adecaro · 2026-04-13T17:30:56Z

@NETIZEN-11 , the linter is still failing. Please, double check. You can run locally make lint and make checks

adecaro · 2026-04-14T04:23:05Z

Hi @NETIZEN-11 , unfortunately, the checks are still reporting issues 😞
Please, make sure all the checks are passing 🙏

NETIZEN-11 · 2026-04-14T15:34:57Z

Hi @adecero,

Thanks for your feedback 🙏
I’ve fixed the linting and formatting issues and verified locally using make lint and make checks.

All checks should be passing now. Please take another look.

Thanks!

AkramBitar · 2026-04-15T07:36:13Z

Hi @NETIZEN-11 , thanks a lot for this PR.

Could you please check the failed test?

adecaro · 2026-04-15T14:21:32Z

the unit-tests pass on my machine, it must be that the test is not tuned for a slower machine like the one the CI is using perhaps. @NETIZEN-11 , please, fix this ASAP 🙏

NETIZEN-11 · 2026-04-16T13:35:29Z

@adecrao Thanks for the review!

Restored version: "2" in .golangci.yml — it was removed unintentionally while fixing local lint issues.
Removed the t.Skipf from manager_test.go and replaced it with proper error handling (require.NoError). Tests will now fail if PostgreSQL doesn't start, as expected.

Let me know if any further changes are needed.

adecaro · 2026-04-16T13:45:52Z

Hi @NETIZEN-11 , I'm having a second pass. I still don't understand what we are gaining. If a go routing is updating, it means that other readers might try to update as well because the cache is empty or not fresh enough. These other go routines then will block on the signal of the updating being completing.

Do you see my point? I'm still wrapping my mind around 😅

adecaro · 2026-04-16T14:01:23Z

yes, if an update is in progress, other readers will satisfy the condition that triggers an update. Now, after the first go routine that acquired the lock completes, these lines (in the original code)

	f.lastFetched = time.Now()
	atomic.StoreUint32(&f.queriesResponded, 0)

guarantee that the other waiting for mu will not need a new db query.
No?

Signed-off-by: Nitesh <nitesh@example.com>

adecaro · 2026-04-28T05:51:25Z

Hi @NETIZEN-11 , I'll review your latest changes ASAP. Sorry for the delay 🙏

adecaro · 2026-04-29T11:56:09Z

Hi @NETIZEN-11 , please, resolve the conflicts 🙏

adecaro · 2026-04-29T12:26:28Z

Hi @NETIZEN-11 , I left a few more comments. Sorry for being picky. This code is on the critical path of a token transaction's lifecycle. We need to make it as clean as possible. Thanks for the understanding 🙏

Signed-off-by: Nitesh Kumar <niteshkumar121411@gmail.com>

Signed-off-by: Nitesh <nitesh@example.com>

NETIZEN-11 · 2026-04-29T19:28:41Z

Hi @adecaro, I’ve addressed all the review comments and pushed the updates. Could you please take another look when you get a chance? Thank u!

adecaro · 2026-04-30T06:20:52Z

Hi @NETIZEN-11 , thanks for the changes. We still have linter issues. Please, run make checks and make lint-auto-fix before pushing again. Thanks much 🙏

adecaro · 2026-05-04T05:14:17Z

Hi @NETIZEN-11 , please, don't forget to fix the lint issues. Thanks 🙏

adecaro · 2026-05-07T14:26:49Z

Hi @NETIZEN-11 , please, let me know if you can continue on this PR. Many thanks 🙏

NETIZEN-11 · 2026-05-08T02:52:11Z

Hi @adecaro, I’ve been dealing with some health issues recently, so it’s taking me a little more time. I’m working on fixing this as soon as possible. Thank you for your patience 🙏

adecaro · 2026-05-08T04:19:52Z

Hi @adecaro, I’ve been dealing with some health issues recently, so it’s taking me a little more time. I’m working on fixing this as soon as possible. Thank you for your patience 🙏

Hi @NETIZEN-11 , your personal health first. Have a speedy recovery 🙏

…te method - Fix type errors in fetcher_test.go: use atomic.StoreInt64() with UnixNano() for lastFetched assignments - Fix undefined function calls: rename newCachedFetcher to NewCachedFetcher in tests - Fix goroutine synchronization: ensure finishUpdate() is called with lock held by acquiring lock before error returns - All tests now pass successfully Signed-off-by: NETIZEN-11 <kumarnitesh979875@gmail.com>

NETIZEN-11 · 2026-05-16T13:05:09Z

Hi @adecaro, I’ve fixed the lint issues and pushed the latest changes. Thank you for your patience and for waiting 🙏 Please let me know if anything else needs to be updated.

adecaro

LGTM

adecaro · 2026-05-19T04:42:48Z

Thanks a lot @NETIZEN-11 for your effort 🙏

NETIZEN-11 force-pushed the fix/sherdlock-update-blocked-readers branch from c05df30 to 3b791af Compare April 12, 2026 19:22

adecaro self-assigned this Apr 13, 2026

adecaro self-requested a review April 13, 2026 05:41

adecaro added the token-selector label Apr 13, 2026

adecaro added this to the Q2/26 milestone Apr 13, 2026

AkramBitar added the enhancement New feature or request label Apr 13, 2026

NETIZEN-11 force-pushed the fix/sherdlock-update-blocked-readers branch 3 times, most recently from 429f175 to 727e53d Compare April 13, 2026 16:54

adecaro approved these changes Apr 13, 2026

View reviewed changes

NETIZEN-11 force-pushed the fix/sherdlock-update-blocked-readers branch 2 times, most recently from 25ab751 to d65a96e Compare April 14, 2026 15:31

adecaro force-pushed the fix/sherdlock-update-blocked-readers branch from d65a96e to d09034e Compare April 14, 2026 15:50

NETIZEN-11 requested a review from adecaro April 15, 2026 18:43

NETIZEN-11 force-pushed the fix/sherdlock-update-blocked-readers branch from ca8232d to f407b2f Compare April 16, 2026 09:18

adecaro reviewed Apr 16, 2026

View reviewed changes

Comment thread .golangci.yml

adecaro reviewed Apr 16, 2026

View reviewed changes

Comment thread token/services/selector/sherdlock/manager_test.go Outdated

adecaro self-requested a review April 16, 2026 17:09

Nitesh added 2 commits April 28, 2026 07:50

fix(sherdlock): restore nolint:gosec directive on context.WithCancel

5cf1ae3

Signed-off-by: Nitesh <nitesh@example.com>

fix(sherdlock): extract finishUpdate helper to remove duplication

c90cc2f

Signed-off-by: Nitesh <nitesh@example.com>

adecaro force-pushed the fix/sherdlock-update-blocked-readers branch from f4b5c51 to c90cc2f Compare April 28, 2026 05:50

adecaro reviewed Apr 29, 2026

View reviewed changes

Comment thread token/services/selector/sherdlock/fetcher.go

adecaro reviewed Apr 29, 2026

View reviewed changes

Comment thread token/services/selector/sherdlock/fetcher.go

adecaro reviewed Apr 29, 2026

View reviewed changes

Comment thread token/services/selector/sherdlock/fetcher.go

adecaro reviewed Apr 29, 2026

View reviewed changes

Comment thread token/services/selector/sherdlock/fetcher.go

adecaro self-requested a review April 29, 2026 12:25

NETIZEN-11 and others added 2 commits April 29, 2026 18:04

Merge branch 'main' into fix/sherdlock-update-blocked-readers

748d2b8

Signed-off-by: Nitesh Kumar <niteshkumar121411@gmail.com>

fix issue

ad4fe17

Signed-off-by: Nitesh <nitesh@example.com>

Merge branch 'main' into fix/sherdlock-update-blocked-readers

bd109cb

NETIZEN-11 force-pushed the fix/sherdlock-update-blocked-readers branch 2 times, most recently from dcbc8df to 119212a Compare May 16, 2026 12:56

NETIZEN-11 force-pushed the fix/sherdlock-update-blocked-readers branch from 119212a to 18e6086 Compare May 16, 2026 12:58

adecaro added 2 commits May 17, 2026 07:33

Merge branch 'main' into fix/sherdlock-update-blocked-readers

09624c1

Merge branch 'main' into fix/sherdlock-update-blocked-readers

5cfa561

adecaro approved these changes May 19, 2026

View reviewed changes

adecaro merged commit 5ae5b75 into hyperledger-labs:main May 19, 2026
94 checks passed

Conversation

NETIZEN-11 commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What was fixed

How it was fixed

Tests added

Uh oh!

NETIZEN-11 commented Apr 12, 2026

Uh oh!

adecaro commented Apr 13, 2026

Uh oh!

adecaro commented Apr 13, 2026

Uh oh!

NETIZEN-11 commented Apr 13, 2026

Uh oh!

adecaro left a comment

Choose a reason for hiding this comment

Uh oh!

adecaro commented Apr 13, 2026

Uh oh!

adecaro commented Apr 14, 2026

Uh oh!

NETIZEN-11 commented Apr 14, 2026

Uh oh!

AkramBitar commented Apr 15, 2026

Uh oh!

adecaro commented Apr 15, 2026

Uh oh!

Uh oh!

Uh oh!

NETIZEN-11 commented Apr 16, 2026

Uh oh!

adecaro commented Apr 16, 2026

Uh oh!

adecaro commented Apr 16, 2026

Uh oh!

adecaro commented Apr 28, 2026

Uh oh!

adecaro commented Apr 29, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adecaro commented Apr 29, 2026

Uh oh!

NETIZEN-11 commented Apr 29, 2026

Uh oh!

adecaro commented Apr 30, 2026

Uh oh!

adecaro commented May 4, 2026

Uh oh!

adecaro commented May 7, 2026

Uh oh!

NETIZEN-11 commented May 8, 2026

Uh oh!

adecaro commented May 8, 2026

Uh oh!

NETIZEN-11 commented May 16, 2026

Uh oh!

adecaro left a comment

Choose a reason for hiding this comment

Uh oh!

adecaro commented May 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

NETIZEN-11 commented Apr 12, 2026 •

edited

Loading